Skip to content

fix(coverage): validate nested npm metadata through canonical pins - #807

Draft
seonghobae wants to merge 24 commits into
mainfrom
fix/npm-nested-metadata-lock-validation
Draft

fix(coverage): validate nested npm metadata through canonical pins#807
seonghobae wants to merge 24 commits into
mainfrom
fix/npm-nested-metadata-lock-validation

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Problem

Central changed-head npm-lock validation currently requires every non-link node_modules location to repeat its own registry tarball and SHA-512 integrity. npm v3 can serialize workspace or peer locations with only version/classification metadata while one canonical package location carries the actual artifact pin. BandScope's valid apps/desktop/node_modules/@types/react-dom entry therefore blocks central coverage even though the canonical root entry is hash-bounded.

Intended focused contract

  • retain exact npm lockfile v2/v3, path, link, HTTPS registry, port, userinfo, query, fragment, tarball, and SHA-512 validation;
  • accept a metadata-only nested location only when one canonical root package has the same scoped/unscoped identity and exact version plus a valid npm-registry tarball and SHA-512 SRI;
  • reject missing canonical metadata, version mismatch, partial pin fields, malformed identities, non-registry URLs, invalid integrity, and conflicting complete pins;
  • consume the validated lock unchanged rather than repairing or inventing metadata;
  • add BandScope-shaped positive evidence and negative identity/version/provenance cases;
  • update APA 7 doctoring and CHANGELOG.md;
  • require complete central statement/branch/docstring evidence.

TDD state

A bounded self-removing workflow on the branch adds the permanent tests, proves the BandScope-shaped case fails against current protected main, applies the minimal production change, executes the full central quality suite, removes itself, and publishes only the four permanent source/test/documentation files. Keep Draft until that workflow-free exact head exists.

Standards basis

The decision follows npm CLI v11's official package-lock.json format, which defines packages as location-keyed descriptors and explicitly notes that some dependency locations may omit artifact fields while retaining metadata. The central policy remains stricter than generic npm by requiring every accepted metadata-only location to resolve to one exact canonical public-registry SHA-512 pin.

Merge gate

No predecessor-head evidence is accepted. Require workflow-free current head, full central tests, 100% production statement/branch/docstring evidence, compilation, Ruff, all security/supply-chain gates, OpenCode, Noema, CodeRabbit, zero actionable threads, a qualifying non-author approval, and branch protection without bypass.

Refs ContextualWisdomLab/bandscope#751 and #748.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a06e795c-4169-4029-b017-f03f0f96328b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@opencode-agent

Implement the bounded GREEN slice on exact current head cf014e8441e8ffbe8b41d93585ceb5c1a3d5bd4a, then remove .github/workflows/materialize-npm-nested-metadata-validation.yml from the final tree. Do not add any other one-shot or branch-writing workflow.

Apply the permanent tests and minimal production changes already specified by the branch contract:

  • accept a metadata-only nested npm v2/v3 node_modules location only when one canonical root package has the same exact package identity/version and one valid public npm-registry .tgz URL plus SHA-512 SRI;
  • reject malformed scoped identities, missing/empty version, partial resolved/integrity, version mismatch, missing canonical metadata, non-registry URL, invalid SRI, and conflicting complete pins;
  • preserve path/link, traversal, userinfo, port, query, fragment, lock-version, exact-head, and immutable-materialization boundaries;
  • consume the validated lock unchanged;
  • add BandScope-shaped positive/negative regressions, APA 7 doctoring, and CHANGELOG evidence;
  • run focused RED/GREEN tests, complete central tests, 100% production statement/branch coverage, 100% public docstrings, Ruff, compilation, and git diff --check.

Keep Draft and do not approve, merge, release, or change reviewer credentials.

Copy link
Copy Markdown
Contributor Author

Release blocker: current branch contains only a trigger and branch-materialization workflows (.github/npm-nested-metadata.trigger, materialize-npm-nested-metadata-validation.yml, trigger-npm-nested-metadata-materializer.yml). Do not run or merge a PR-controlled workflow that rewrites and pushes its own reviewed source.

@opencode-agent address

Remove every trigger, self-removing materializer, encoded patch, and branch-writing helper from the final tree. Preserve the test-first intent, but publish the four permanent implementation/test/doctoring/changelog files through ordinary reviewed commits. The final production rule may accept a metadata-only nested npm v2/v3 location only when one canonical root package with the same normalized identity and exact version has a valid public npm-registry tarball and SHA-512 SRI. Continue to reject missing canonical metadata, version mismatch, partial pins, malformed scoped identities, non-registry origins, userinfo/query/fragment/port abuse, invalid SRI, conflicting complete pins, unsafe paths, and links. Consume the lock unchanged; do not repair or synthesize metadata. Require exact-head Python 3.10/3.14 tests, 100% production statement/branch/docstring evidence, compilation, all security/supply-chain checks, current-head review, and independent approval before Ready.

Copy link
Copy Markdown
Contributor Author

@jules Replace the trigger/materializer-only scope on exact live head 4b2fceea5cbbff929623fec85b371f7f0bbe2898 with ordinary reviewed permanent implementation, tests, APA 7 doctoring, and CHANGELOG commits. Keep Draft.

Delete .github/npm-nested-metadata.trigger, .github/workflows/materialize-npm-nested-metadata-validation.yml, and .github/workflows/trigger-npm-nested-metadata-materializer.yml; do not add another branch writer, encoded patch, trigger, source snapshot, or temporary workflow.

Implement the narrow npm v2/v3 rule directly: a metadata-only nested node_modules location is accepted only when exactly one canonical root package has the same normalized scoped/unscoped identity and exact nonempty version, a valid https://registry.npmjs.org/...tgz URL, and one valid SHA-512 SRI. Reject missing or ambiguous canonical metadata, version mismatch, partial pins, malformed identities, conflicting complete pins, non-registry origin, port/userinfo/query/fragment abuse, invalid integrity, unsafe path, or link. Consume the original lock unchanged; never repair or synthesize metadata.

Add the BandScope-shaped positive case and every stated negative case. Run Python 3.10/3.14, focused and complete central tests, 100% production statement/branch/public-docstring coverage, Ruff, compileall, workflow contract, security and supply-chain checks, and git diff --check. Commit normally; no amend, force-push, Ready transition, approval, merge, release, reviewer-credential change, or protection change. Stop without mutation if the live head differs before writing.

Comment thread .github/workflows/pr807-apply-nested-metadata-once.yml Fixed

Copy link
Copy Markdown
Contributor Author

/oc

Fix the current exact-head failure directly in permanent source; do not rely on, add, or retain a one-shot/branch-writer workflow.

The Python 3.14 quality run proves 12 focused failures because scripts/ci/materialize_base_javascript_packages.py still executes the old rule that every non-link node_modules location must carry local resolved and integrity. Apply the reviewed logic currently represented by scripts/ci/apply_pr807_nested_metadata.py:

  • derive an exact scoped or unscoped package identity after the final node_modules segment;
  • keep complete local pins subject to the existing HTTPS npm-registry URL and SHA-512 SRI checks;
  • reject a partial resolved/integrity pair;
  • allow a metadata-only nested location only with a nonempty exact version and one canonical node_modules/<identity> non-link entry whose exact version matches and whose complete registry/SRI pin validates;
  • reject metadata-only canonical-root entries, malformed identities, missing/linked canonical entries, mismatched versions, hostile origins, and invalid integrity;
  • preserve lockfile-v2/v3, path, traversal, workspace-link, userinfo, port, query, fragment, tarball, and unchanged-lock-byte boundaries.

Add the APA 7 doctoring and CHANGELOG entry, remove scripts/ci/apply_pr807_nested_metadata.py and .github/workflows/pr807-apply-nested-metadata-once.yml from the final tree, and keep only the permanent quality workflow. Run the two focused suites with 100% branch coverage, production docstrings, Python 3.10/3.14 compilation, complete central tests/coverage, and git diff --check. Keep Draft until the workflow-free exact head is green.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

The branch has been restored to a reviewable TDD state at exact head 0ce11b73992851afe991a5e86991db398c9d9900: every PR-controlled writer, trigger, one-shot materializer, and patch applicator has been deleted. Do not recreate any source-writing workflow.

Implement the permanent change directly in scripts/ci/materialize_base_javascript_packages.py through an ordinary reviewed commit. Preserve every existing v2/v3 path, workspace-link, HTTPS public-registry, port/userinfo/query/fragment, SHA-512 SRI, and unchanged-lock boundary. Required contract:

  • parse each non-link node_modules location into one exact scoped or unscoped package identity from the suffix after its last node_modules component;
  • a complete location must still validate its own resolved and integrity exactly as today;
  • a metadata-only location may omit both fields only when it has a nonempty exact string version and one canonical root key node_modules/<identity> has the same version plus a complete valid public-registry tarball and SHA-512 pin;
  • reject canonical root entries that are themselves metadata-only, missing, malformed, version-mismatched, or partial;
  • reject any location declaring exactly one of resolved/integrity as must not partially declare;
  • preserve independently complete nested pins even when their version differs from the canonical root;
  • reject malformed scoped identities such as @scope without a package and extra components after a scoped package;
  • consume the lock unchanged; do not repair or synthesize fields.

Keep tests/test_npm_nested_metadata_lock_validation.py authoritative, add complete statement/branch/docstring coverage in the existing permanent quality workflow, update APA 7 doctoring and CHANGELOG.md through ordinary commits, then rerun every exact-head quality/security/supply-chain gate. Keep Draft and merge-block labels until GREEN and independent approval.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Implement the permanent production change on exact current Draft head 0ce11b73992851afe991a5e86991db398c9d9900. Quality run 31139010086 proves Python 3.10 compilation and every security/supply-chain gate are green, while the Python 3.14 focused job has 12 RED failures because the branch currently contains only the quality workflow and tests; scripts/ci/materialize_base_javascript_packages.py is unchanged.

Apply the minimal two-pass validator:

  1. Parse and validate every package location/path and workspace link exactly as today.
  2. Derive the npm package identity from the suffix after the last node_modules component:
    • unscoped identity: exactly one nonempty segment;
    • scoped identity: exactly two nonempty segments, first beginning with @;
    • reject incomplete or extra-segment identities as malformed npm package identity.
  3. Classify each non-link location:
    • complete pin: version, resolved, and integrity all present; validate nonempty version, exact public npm HTTPS tarball provenance, no port/userinfo/query/fragment, and one SHA-512 SRI;
    • metadata-only: none of resolved/integrity present; require a nonempty exact version;
    • partial pin: exactly one of resolved/integrity present; fail with must not partially declare.
  4. Build canonical pins only from complete root locations of the exact form node_modules/<identity>—not nested copies. Preserve independently complete nested pins as independently validated artifacts.
  5. For each metadata-only location, require exactly one safe canonical root pin with the same package identity and exact version. Fail separately for missing canonical root pin and version mismatch.
  6. Do not repair, synthesize, reorder, or rewrite the lockfile; validation only.

Add scripts/ci/materialize_base_javascript_packages.py, APA 7 doctoring, and CHANGELOG.md to the permanent PR scope and quality-workflow triggers. Keep the branch workflow-free except the permanent read-only quality gate; no self-removing or write-capable patcher may return.

Run:

python -m coverage erase
python -m coverage run --branch -m pytest -q \
  tests/test_materialize_base_javascript_packages.py \
  tests/test_npm_nested_metadata_lock_validation.py
python -m coverage report \
  --include=scripts/ci/materialize_base_javascript_packages.py \
  --show-missing --fail-under=100
python -m interrogate scripts/ci
python -m compileall -q scripts/ci tests
pytest -q

Then rerun exact-head CodeQL, Python Security, SAST, Security Scan, Secret Scan, OSV, Scorecard, SBOM, CodeRabbit/OpenCode/Noema, and keep Draft until all evidence is current and a qualifying independent approval exists.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Exact head 0ce11b73992851afe991a5e86991db398c9d9900 has Python 3.10 compatibility and every security/supply-chain gate green. Python 3.14 focused quality is intentionally RED: 44 passed, 12 failed, all because validate_head_npm_lock() still requires every non-link node_modules location to repeat resolved and integrity.

Implement the minimum permanent production correction directly in scripts/ci/materialize_base_javascript_packages.py; do not reintroduce a PR-controlled source-rewriting workflow.

  • Derive package identity from the final node_modules segment. An unscoped identity is exactly one following segment; a scoped identity is exactly @scope/name. Reject an empty scope/name, @scope without a package, or any extra path segment after the identity as malformed npm package identity.
  • Classify artifact fields exactly: both resolved and integrity = complete pin; neither = metadata-only; exactly one = fail with must not partially declare.
  • Continue validating every complete pin with the existing closed public npm-registry HTTPS/tarball/SHA-512 contract.
  • Build canonical root pins only from exact top-level paths node_modules/name and node_modules/@scope/name. A root canonical entry must be a complete validated pin; a metadata-only root entry fails with canonical root pin.
  • A metadata-only nested location must have one nonempty exact string version, resolve to a complete validated canonical root pin with the same package identity, and match its exact version. Otherwise fail with the committed canonical root pin, nonempty exact version, or exact canonical version contract.
  • A fully and independently pinned nested package remains valid even when a canonical root package with the same identity has a different version. Never synthesize, repair, or mutate lock metadata.
  • Preserve link/path safety, lockfile v2/v3, registry URL, port/userinfo/query/fragment, tarball identity, integrity, bounded parsing, and fail-closed behavior.

Add the permanent production tests to the authoritative existing module, complete public docstrings, APA 7 doctoring and CHANGELOG.md; update the stale PR body so it no longer claims a self-removing workflow. Run focused 100% production statement/branch/docstring coverage, Python 3.10/3.14 compilation, the complete central suite, actionlint, and all security/supply-chain gates on one unchanged head. Keep Draft until GREEN.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Implement the permanent GREEN production change for exact head 0ce11b73992851afe991a5e86991db398c9d9900 directly through ordinary reviewed commits. Do not add a trigger, materializer, repair, self-removing, encoded-patch, or branch-writing workflow.

Update scripts/ci/materialize_base_javascript_packages.py::validate_head_npm_lock test-first so npm v2/v3 metadata-only nested locations are accepted only through one exact canonical root pin:

  1. Parse and validate every node_modules path first. Derive the package identity from the segments after the final node_modules: exactly one unscoped segment or exactly @scope/name; reject incomplete scopes and extra segments as malformed npm package identity.
  2. Require every non-link package entry to have a nonempty string version.
  3. Classify resolved/integrity as both absent, both present, or partial. Reject partial declarations as must not partially declare.
  4. Validate every complete pin with the existing exact HTTPS registry URL, port/userinfo/query/fragment/tarball, and SHA-512 SRI controls.
  5. A root entry at exactly node_modules/<identity> must be a complete pin; a metadata-only root entry fails with canonical root pin.
  6. A metadata-only nested entry may inherit only the complete canonical root pin for the same identity and exact version. Reject missing root pins and version mismatches with the permanent contract messages.
  7. Independently complete nested pins remain valid, including a different nested version.
  8. Preserve link handling, path safety, lockfile v2/v3 constraints, and all pre-existing tests.

Then add authoritative npm v11 package-lock doctoring with APA 7 primary-source references and CHANGELOG.md, include implementation/docs/changelog in the permanent quality workflow path filters, run focused tests with 100% production statement/branch coverage and docstrings, run the complete central suite, compile, remove all transient scope, and leave the PR Draft until the workflow-free exact head is green.

Copy link
Copy Markdown
Contributor Author

/opencode

Continue directly on fix/npm-nested-metadata-lock-validation at exact RED head 0ce11b73992851afe991a5e86991db398c9d9900; do not open a stacked PR, do not update main, and do not add or restore any PR-controlled patcher, self-removing workflow, encoded patch, or branch writer.

The permanent tests are present and the unsafe patcher has been removed. Implement the smallest ordinary source/docs/changelog change in scripts/ci/materialize_base_javascript_packages.py so the focused RED contract becomes GREEN:

  1. Derive an exact npm package identity only from the segments after the final node_modules: one non-scoped segment or exactly @scope/name. Reject malformed identities.
  2. Preserve existing lockfile v2/v3, repository-relative path, workspace-link, HTTPS registry.npmjs.org, no userinfo/port/query/fragment, .tgz, and single SHA-512 SRI validation.
  3. For a non-link installed location with both resolved and integrity, validate its complete pin independently as today. Reject partial pin fields.
  4. For a metadata-only nested location, require a nonempty exact version, forbid metadata-only canonical root locations, and resolve only to node_modules/<same exact identity> in the same authenticated lock document. The canonical entry must be a non-link mapping with the identical exact version and a valid public-registry tarball/SHA-512 pair.
  5. Do not repair, infer, fetch, rewrite, or mutate the lock; reject missing canonical metadata, version mismatch, malformed identity, hostile registry URL, invalid SRI, linked canonical entries, and conflicting/partial pins.
  6. Add the reviewed APA 7 doctoring and CHANGELOG entry as permanent files, not generated at runtime.

Run tests/test_npm_nested_metadata_lock_validation.py first with complete production branch coverage, then the complete central regression, docstring, compilation, Ruff/actionlint, and security gates. Commit only the permanent implementation, tests, doctoring, changelog, and focused quality workflow directly to this branch. Keep Draft until the exact new head is fully green.

Copy link
Copy Markdown
Contributor Author

@opencode-agent Fix the current exact-head quality failure on 0c749645279b9109f53a3e9ac4ae23c962aa93a1 without weakening the canonical-pin policy.

Run 31141366307, job 92751790022, has exactly four focused failures. All are legacy error-precedence regressions: validate_head_npm_lock() requires version before validating an already supplied resolved/integrity pair, so invalid origin, invalid SRI, malformed port, and an empty canonical entry all report must declare a nonempty exact version instead of the established bounded errors.

Required repair:

  • for a non-link location, compute has_resolved/has_integrity before requiring version;
  • reject partial pin fields first;
  • when both pin fields exist, run _validate_npm_registry_pin() before requiring a nonempty version, preserving invalid URL/origin/SHA-512 diagnostics;
  • still require a nonempty exact version before registering any complete canonical root or accepting any metadata-only nested location;
  • for a canonical root with neither pin fields nor version, preserve the established must pin a registry tarball and SHA-512 integrity failure rather than treating it as an eligible metadata-only location;
  • retain all BandScope-shaped positive cases, scoped/unscoped identity rules, exact-version equality, unchanged lock bytes, URL restrictions, 100% statement/branch/docstring coverage, Python 3.10 compatibility, and the five-file permanent scope;
  • add no temporary, self-removing, generated-patch, or branch-writing workflow.

Keep Draft until fresh exact-head quality/security/review/approval gates pass. Do not approve, merge, tag, release, or publish.

Copy link
Copy Markdown
Contributor Author

/oc

Close the remaining exact-head coverage deficit on 675994f713c2e4da5fd740d5de2fd9670cf406de without weakening or excluding production code. The focused behavior is green (56 tests), but materialize_base_javascript_packages.py is 99% with uncovered line/partial branches 63->49, 210->229, 222->211, and 281.

Add focused executable tests that exercise each real fail-closed/loop-control path, including the remaining malformed/empty package identity path, canonical-location iteration paths that skip ineligible/link/version-mismatched candidates before a later valid candidate or terminal failure, and the uncovered nested-entry rejection at line 281. Where an arc is demonstrably impossible after an earlier invariant, simplify the production control flow rather than adding pragma: no cover; retain the same error contract and resource bounds.

Then run the focused suite under branch coverage with 100% required, production docstrings, Python 3.10/3.14 compilation, complete central tests/coverage, and git diff --check. Keep the five-file workflow-free scope; do not reintroduce helpers or one-shot writers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants